Skip to content

Conversation

lannuttia
Copy link
Contributor

@lannuttia lannuttia commented Oct 15, 2025

Description

This change attempts to make the click environment sniffing more robust by using loaded modules to sniff out whether it is being ran as part of a server or not.

Fixes #3846

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

This has been validated with automated tests that are included in this pull request.

Does This PR Require a Core Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@lannuttia lannuttia force-pushed the improve-click-environment-sniffing branch from e70ac33 to ba73170 Compare October 15, 2025 04:00
@lannuttia lannuttia requested a review from a team as a code owner October 15, 2025 04:00
@lannuttia lannuttia force-pushed the improve-click-environment-sniffing branch 2 times, most recently from 0b1d304 to cb89a34 Compare October 15, 2025 04:06
@xrmx xrmx moved this to Reviewed PRs that need fixes in @xrmx's Python PR digest Oct 15, 2025
@lannuttia
Copy link
Contributor Author

One additional thing that I'd like to point out is I still don't think I'm super happy with how this works. I don't really like how I can't think of a way to test this behavior without mocking and effectively coupling the test to the implementation. I'm doing it this way primarily because I see it as being the shortest path to more robustly sniffing out if we shouldn't be running the click instrumentation.

I have no idea how large of a lift this would be but it might be nice to provide a way for instrumentation to disable other instrumentation. If we were to add something like that as part of the BaseInstrumentor class and determine what instrumentation should and should not be enabled based on something like a property of that class, I think that would lead to things being more testable. Then we don't have to couple the test here to the implementation by mocking sys.modules. We can have the test just ensure that the property is what we expect it to be and trust that whatever's responsible for honoring that, does.

@lannuttia lannuttia force-pushed the improve-click-environment-sniffing branch 2 times, most recently from 0f485d1 to 4abadf1 Compare October 16, 2025 02:28
@lannuttia lannuttia force-pushed the improve-click-environment-sniffing branch from 4abadf1 to 59fca1f Compare October 16, 2025 02:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Reviewed PRs that need fixes

Development

Successfully merging this pull request may close these issues.

Click Instrumentation Bypass Issue

2 participants